Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-93910: micro optimise enum attribute access #94920

Closed
wants to merge 1 commit into from

Conversation

hauntsaninja
Copy link
Contributor

@hauntsaninja hauntsaninja commented Jul 17, 2022

This halves access to attributes such as methods or _sunder_ name

Benchmarked using:

from enum import Enum

class Color(Enum):
    RED = "Red"
    BLUE = "Blue"
    GREEN = "Green"

def f():
    for _ in range(1000):
        Color.RED
        Color.BLUE
        Color.GREEN

def g():
    for _ in range(1000):
        Color.RED._name_
        Color.BLUE._name_
        Color.GREEN._name_

import timeit
print(timeit.timeit('f()', number=10000, globals=globals()))
print(timeit.timeit('g()', number=10000, globals=globals()))

As of today morning:

6da988a46c λ ./python.exe bm_enum.py
1.499613167019561
1.5516562079428695

As of the latest enum change #94913:

c20186c397 λ ./python.exe bm_enum.py
1.4998537090141326
16.095267291995697

As of this PR:

gh-93910-micro λ ./python.exe bm_enum.py
1.4983433339511976
8.81327024998609

This halves access to attributes such as methods or _sunder_ name

Benchmarked using:
```
from enum import Enum

class Color(Enum):
    RED = "Red"
    BLUE = "Blue"
    GREEN = "Green"

def f():
    for _ in range(1000):
        Color.RED
        Color.BLUE
        Color.GREEN

def g():
    for _ in range(1000):
        Color.RED._name_
        Color.BLUE._name_
        Color.GREEN._name_

import timeit
print(timeit.timeit('f()', number=10000, globals=globals()))
print(timeit.timeit('g()', number=10000, globals=globals()))
```

As of today morning:
```
6da988a λ ./python.exe bm_enum.py
1.499613167019561
1.5516562079428695
```

As of the latest enum change python#94913:
```
c20186c λ ./python.exe bm_enum.py
1.4998537090141326
16.095267291995697
```

As of this PR:
```
pythongh-93910-micro λ ./python.exe bm_enum.py
1.4983433339511976
8.81327024998609
```
@hauntsaninja hauntsaninja added 3.11 only security fixes 3.12 bugs and security fixes skip news labels Jul 17, 2022
@hauntsaninja hauntsaninja marked this pull request as draft July 17, 2022 02:51
@hauntsaninja hauntsaninja marked this pull request as ready for review July 17, 2022 03:07
@hauntsaninja
Copy link
Contributor Author

hauntsaninja commented Jul 17, 2022

Hmm, actually given that _member_names_ is a list, this is a little expensive for moderate to large enums (equals current main at >50 members). So maybe we throw this change away.

Note that there's a micro-optimisation we can do where we preserve condition order, but don't get self_dict if we don't need it:

- if isinstance(value, cls) and name not in self_dict and name in self._member_names_:
+ if isinstance(value, cls) and name not in super().__getattribute__('__dict__') and name in self._member_names_:

This reduces benchmark time to 12s (from 16s on main, compared to 1.5s yesterday) and should be a strict improvement regardless of enum size.

@hauntsaninja hauntsaninja changed the title gh-93910: optimise enum attribute access gh-93910: micro optimise enum attribute access Jul 17, 2022
@ethanfurman
Copy link
Member

Thank you for the effort, but it has been decided to not use __getattribute__.

@hauntsaninja hauntsaninja deleted the gh-93910-micro branch July 18, 2022 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes awaiting review skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants